Add missing `methodtools` dependency of the `common-sql` provider by tatiana · Pull Request #47148 · apache/airflow

tatiana · 2025-02-27T14:15:08Z

Since apache-airflow-providers-common-sql==1.23.0, released on 26 February 2025, users started facing the error:

    from cosmos.operators._asynchronous.bigquery import DbtRunAirflowAsyncBigqueryOperator
cosmos/operators/_asynchronous/bigquery.py:8: in <module>
    from airflow.providers.google.cloud.operators.bigquery import BigQueryInsertJobOperator
../../../.local/share/hatch/env/virtual/astronomer-cosmos/Za_bFbg4/tests.py3.12-2.9/lib/python3.12/site-packages/airflow/providers/google/cloud/operators/bigquery.py:32: in <module>
    from airflow.providers.common.sql.operators.sql import (  # type: ignore[attr-defined] # for _parse_boolean
../../../.local/share/hatch/env/virtual/astronomer-cosmos/Za_bFbg4/tests.py3.12-2.9/lib/python3.12/site-packages/airflow/providers/common/sql/operators/sql.py:[29](https://github.com/astronomer/astronomer-cosmos/actions/runs/13544957799/job/37854234771#step:6:30): in <module>
    from airflow.providers.common.sql.hooks.sql import DbApiHook, fetch_all_handler, return_single_query_results
../../../.local/share/hatch/env/virtual/astronomer-cosmos/Za_bFbg4/tests.py3.12-2.9/lib/python3.12/site-packages/airflow/providers/common/sql/hooks/sql.py:37: in <module>
    from methodtools import lru_cache
E   ModuleNotFoundError: No module named 'methodtools'

When trying to import:

from airflow.providers.google.cloud.operators.bigquery import BigQueryInsertJobOperator

It seems this started happening since #41327.

This PR aims to solve this issue.

Closes: #47147

dabla · 2025-02-27T15:11:05Z

Indeed since 41327 I've added the lru cache for use with dialects. Isn't there a way to detect such missing dependency when running the tests of the provider?

gopidesupavan · 2025-02-27T15:32:32Z

I believe we moved adding dependency to provider info and pyproject.toml files, so IMHO It would be better to add this dependency to pyproject.toml https://github.com/apache/airflow/blob/main/providers/common/sql/pyproject.toml#L57

Then the pre-commit will add that to readme and provider info file https://github.com/apache/airflow/blob/main/providers/common/sql/src/airflow/providers/common/sql/get_provider_info.py

potiuk · 2025-02-27T16:00:08Z

I believe we moved adding dependency to provider info and pyproject.toml files, so IMHO It would be better to add this dependency to pyproject.toml https://github.com/apache/airflow/blob/main/providers/common/sql/pyproject.toml#L57

Yep. This is the new (much more standard) way how we should add dependencies. In fact I am a little puzzled why this PR did not fail with validation of the provider.yaml, because ... it should ... We do not exapect dependencies field in provider.yaml any more. Let me see why it did not fail.

potiuk · 2025-02-27T16:04:22Z

OK. I know ... Missing case for selective checks

potiuk · 2025-02-27T16:27:19Z

The #47148 improves selective checks to cover that case.

potiuk

Requesting changes to avoid accidental merge.

Since `apache-airflow-providers-common-sql==1.23.0`, released on 26 February 2025, users started facing the error: ``` from cosmos.operators._asynchronous.bigquery import DbtRunAirflowAsyncBigqueryOperator cosmos/operators/_asynchronous/bigquery.py:8: in <module> from airflow.providers.google.cloud.operators.bigquery import BigQueryInsertJobOperator ../../../.local/share/hatch/env/virtual/astronomer-cosmos/Za_bFbg4/tests.py3.12-2.9/lib/python3.12/site-packages/airflow/providers/google/cloud/operators/bigquery.py:32: in <module> from airflow.providers.common.sql.operators.sql import ( # type: ignore[attr-defined] # for _parse_boolean ../../../.local/share/hatch/env/virtual/astronomer-cosmos/Za_bFbg4/tests.py3.12-2.9/lib/python3.12/site-packages/airflow/providers/common/sql/operators/sql.py:[29](https://github.com/astronomer/astronomer-cosmos/actions/runs/13544957799/job/37854234771#step:6:30): in <module> from airflow.providers.common.sql.hooks.sql import DbApiHook, fetch_all_handler, return_single_query_results ../../../.local/share/hatch/env/virtual/astronomer-cosmos/Za_bFbg4/tests.py3.12-2.9/lib/python3.12/site-packages/airflow/providers/common/sql/hooks/sql.py:37: in <module> from methodtools import lru_cache E ModuleNotFoundError: No module named 'methodtools' ``` It seems this started happening since apache#41327. This PR aims to solve this issue. Closes: apache#47147

tatiana · 2025-02-27T18:05:57Z

Thank you very much, @dabla, @gopidesupavan, and @potiuk, for the rapid reviews and feedback!

I moved the dependency from provider.yaml to pyproject.toml in e7a6619.

I noticed the following files were modified after running the pre-commit checks:

generated/provider_dependencies.json
providers/common/sql/README.rst
providers/common/sql/src/airflow/providers/common/sql/get_provider_info.py

For now, I have added them to the PR. Please let me know if that's correct - I can remove them if necessary.

potiuk · 2025-02-27T18:24:00Z

For now, I have added them to the PR. Please let me know if that's correct - I can remove them if necessary.

Yep. This is exactly what pre-commits are doing - take your change and apply whatever resulting changes elsewhere are needed so that you do not have to even think about it.

dabla · 2025-02-27T19:05:01Z

Awesome to see how quickly this issue has been resolved 👍

potiuk · 2025-02-27T20:10:08Z

The static-checks error is likely from main

potiuk · 2025-02-27T20:24:06Z

The static check failure handled in #47172

…ache#47148) * Add methodtools as a dependency of the common-sql provider Since `apache-airflow-providers-common-sql==1.23.0`, released on 26 February 2025, users started facing the error: ``` from cosmos.operators._asynchronous.bigquery import DbtRunAirflowAsyncBigqueryOperator cosmos/operators/_asynchronous/bigquery.py:8: in <module> from airflow.providers.google.cloud.operators.bigquery import BigQueryInsertJobOperator ../../../.local/share/hatch/env/virtual/astronomer-cosmos/Za_bFbg4/tests.py3.12-2.9/lib/python3.12/site-packages/airflow/providers/google/cloud/operators/bigquery.py:32: in <module> from airflow.providers.common.sql.operators.sql import ( # type: ignore[attr-defined] # for _parse_boolean ../../../.local/share/hatch/env/virtual/astronomer-cosmos/Za_bFbg4/tests.py3.12-2.9/lib/python3.12/site-packages/airflow/providers/common/sql/operators/sql.py:[29](https://github.com/astronomer/astronomer-cosmos/actions/runs/13544957799/job/37854234771#step:6:30): in <module> from airflow.providers.common.sql.hooks.sql import DbApiHook, fetch_all_handler, return_single_query_results ../../../.local/share/hatch/env/virtual/astronomer-cosmos/Za_bFbg4/tests.py3.12-2.9/lib/python3.12/site-packages/airflow/providers/common/sql/hooks/sql.py:37: in <module> from methodtools import lru_cache E ModuleNotFoundError: No module named 'methodtools' ``` It seems this started happening since apache#41327. This PR aims to solve this issue. Closes: apache#47147 * Fix linting * Revert change to provider.yaml and applyc hange to pyproject.toml

tatiana requested a review from eladkal as a code owner February 27, 2025 14:15

boring-cyborg bot added the provider:common-sql label Feb 27, 2025

This was referenced Feb 27, 2025

Disable example_cosmos_dbt_build.py DAG in CI astronomer/astronomer-cosmos#1567

Merged

Revert installing methodtools as a dependency of Cosmos astronomer/astronomer-cosmos#1572

Open

dabla approved these changes Feb 27, 2025

View reviewed changes

potiuk requested changes Feb 27, 2025

View reviewed changes

tatiana changed the title ~~Add methodtools as a dependency of the common-sql provider~~ Add missing methodtools dependency of the common-sql provider Feb 27, 2025

tatiana added 3 commits February 27, 2025 18:04

Fix linting

20e3a23

Revert change to provider.yaml and applyc hange to pyproject.toml

e7a6619

tatiana force-pushed the fix-common-sql-methodtools branch from 427189e to e7a6619 Compare February 27, 2025 18:04

potiuk approved these changes Feb 27, 2025

View reviewed changes

gopidesupavan approved these changes Feb 27, 2025

View reviewed changes

potiuk merged commit c0719fe into apache:main Feb 27, 2025
145 of 146 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add missing `methodtools` dependency of the `common-sql` provider#47148

Add missing `methodtools` dependency of the `common-sql` provider#47148
potiuk merged 3 commits intoapache:mainfrom
tatiana:fix-common-sql-methodtools

tatiana commented Feb 27, 2025 •

edited

Loading

Uh oh!

dabla commented Feb 27, 2025

Uh oh!

gopidesupavan commented Feb 27, 2025 •

edited

Loading

Uh oh!

potiuk commented Feb 27, 2025

Uh oh!

potiuk commented Feb 27, 2025

Uh oh!

potiuk commented Feb 27, 2025

Uh oh!

potiuk left a comment

Uh oh!

tatiana commented Feb 27, 2025

Uh oh!

potiuk commented Feb 27, 2025

Uh oh!

dabla commented Feb 27, 2025

Uh oh!

potiuk commented Feb 27, 2025

Uh oh!

potiuk commented Feb 27, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

tatiana commented Feb 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dabla commented Feb 27, 2025

Uh oh!

gopidesupavan commented Feb 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

potiuk commented Feb 27, 2025

Uh oh!

potiuk commented Feb 27, 2025

Uh oh!

potiuk commented Feb 27, 2025

Uh oh!

potiuk left a comment

Choose a reason for hiding this comment

Uh oh!

tatiana commented Feb 27, 2025

Uh oh!

potiuk commented Feb 27, 2025

Uh oh!

dabla commented Feb 27, 2025

Uh oh!

potiuk commented Feb 27, 2025

Uh oh!

potiuk commented Feb 27, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

tatiana commented Feb 27, 2025 •

edited

Loading

gopidesupavan commented Feb 27, 2025 •

edited

Loading